chore: refactor spam labels #2151
Conversation
Up to standards ✅🟢 Issues
|
WalkthroughAdded constants Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/scripts/update-spam-list.js (1)
219-232: 🛠️ Refactor suggestion | 🟠 MajorPromote
'automated'label to a top-level constant.One label value is still hardcoded in the changed path; keep all label config centralized.
♻️ Proposed refactor
const LABEL_SPAM = 'notes: spam'; const LABEL_SPAM_LIST_UPDATE = 'notes: spam-list-update'; +const LABEL_AUTOMATED = 'automated'; @@ - labels: [LABEL_SPAM_LIST_UPDATE, 'automated'] + labels: [LABEL_SPAM_LIST_UPDATE, LABEL_AUTOMATED]As per coding guidelines, "Use top-level constants for configuration — avoid hardcoded values scattered through the script."
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6d43ad67-78b6-4a61-a688-04af1105ba6c
📒 Files selected for processing (1)
.github/scripts/update-spam-list.js
5ef0350 to
1526f29
Compare
|
Rebase please @Gitjay11 or hit update branch |
f13609b
1526f29 to
f13609b
Compare
|
@exploreriii I did it now. |
|
Hi @Gitjay11, please update the branch |
Signed-off-by: Ajay Rajera <newajay.11r@gmail.com>
f13609b to
4d6b924
Compare
@manishdait I did it again |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/scripts/cron-admin-update-spam-list.js (1)
1-8:⚠️ Potential issue | 🟡 MinorUpdate the module docstring to reflect the renamed label.
Line 5 still references the old
'spam'label. Since this PR standardizes onnotes: spam, update the header comment accordingly to stay consistent with the new constants and the issue#2129intent.📝 Proposed doc tweak
- * - Identifies spam users from closed unmerged PRs with 'spam' label + * - Identifies spam users from closed unmerged PRs with the 'notes: spam' label
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 060ea822-edcc-4665-ad58-22b9492424a8
📒 Files selected for processing (1)
.github/scripts/cron-admin-update-spam-list.js
Signed-off-by: Ajay Rajera <newajay.11r@gmail.com>
Signed-off-by: Ajay Rajera <newajay.11r@gmail.com>
Signed-off-by: Ajay Rajera <newajay.11r@gmail.com>
Description:
Refactor the GitHub spam list automation script to use the newly standardized label names for spam tracking and issue creation. To make future renaming effortless, the label strings have been extracted into central constants at the top of the module.
LABEL_SPAMandLABEL_SPAM_LIST_UPDATEconstants to the spam list updater scriptspamlabel to query"notes: spam"with proper syntax quotingspam-list-updatetonotes: spam-list-updateRelated issue(s):
Fixes #2129
Notes for reviewer:
N/A - This is an internal CI/automation script refactor. The search query string syntax was specifically verified to properly handle the spaces inside
notes: spamduring graph queries.Checklist